Feature: Auto Persisted Queries with CDN Support #608

Codebear98 · 2019-07-02T10:27:48Z

Usage

let networkTransport = HTTPNetworkTransport(
      url: url,
      configuration: sessionConfiguration,
      useGETForQueries: false,
      enableAutoPersistedQueries: true,
      useHttpGetMethodForPersistedQueries: true)

let client = ApolloClient(networkTransport: networkTransport)

Features

Support APQs
Support use HTTPGetMethod for all APQs Requests by setting useGETForQueries=true
Support use HTTPGetMethod only for supported APQs request by setting useGETMethodForPersistedQueries=true , according to https://www.apollographql.com/docs/apollo-server/features/apq/#cdn

useGETMethodForPersistedQueries=true is aligning with apollo-android #1376

it sends GetMethod for the 1st Request.
If server dose not have cache or not support APQs with error message PersistedQueryNotFound or PersistedQueryNotSupported
Then it fallbacks to retry with POST Request (retry with GET when useGETForQueries=true) and includes QueryDocument for registering APQs.

useGETMethodForPersistedQueries is introduced due to an error exceeds limit on url query string may occur on some sever when query string exceeds 2k limit with large amount of data such as QueryDoucment.

Limitation

To enable APQs,sha256hash of QueryDocument is required for each QueryOperation.
You need to Modify Apollo runscript in YOUR PROJECT to also generate operationIdentifier (sha256hash) to your API.swift (please see below)
Must use Apollo 1.9.2, since there is a bug in Apollo-tooling while generating operationIdentifier
The Query and hash does not match when the Query contains fragment. It's due to an extra \n added in QueryDocument for hashing. But the \n is missing in Swift CodeGen.
operationId is invalid for swift codegen apollo-tooling#1362

There is a hacking workaround in HTTPNetworkTransport.swift line212, It adds back the \n to the QueryDocument.

if sendQueryDocument {
    // TODO: This work-around fix "operationId is invalid for swift codegen" (https://github.com/apollographql/apollo-tooling/issues/1362), please remove this work-around after it's fixed.
    let modifiedQuery = operation.queryDocument.replacingOccurrences(of: "fragment", with: "\nfragment")
    payload["query"] = modifiedQuery
  }

Apollo runscript
add --operationIdsPath=operationIdsPath for swift codegen


# Type a script or drag a script file from your workspace to insert its path.
APOLLO_FRAMEWORK_PATH=$(eval find $FRAMEWORK_SEARCH_PATHS -name "Apollo.framework" -maxdepth 1 -print | head -n 1)

if [ -z "$APOLLO_FRAMEWORK_PATH" ]; then
echo "error: Couldn't find Apollo.framework in FRAMEWORK_SEARCH_PATHS; make sure to add the framework to your project."
exit 1
fi

# Download schema.json first 
# apollo schema:download --endpoint=http... --header="apikey: ..."
cd "${SRCROOT}/${TARGET_NAME}/DataSource/GraphQL"
$APOLLO_FRAMEWORK_PATH/check-and-run-apollo-cli.sh codegen:generate --passthroughCustomScalars --queries="$(find . -name '*.graphql')" --operationIdsPath=operationIdsPath --schema=schema.json Generated/GraphQLAPI.swift

designatednerd · 2019-07-02T13:18:31Z

@Codebear98 we've got some changes in progress with this with #599 and #602 - you may want to hold off until those get merged!

Codebear98 · 2019-07-03T03:29:52Z

@Codebear98 we've got some changes in progress with this with #599 and #602 - you may want to hold off until those get merged!

cool, make sense for #599, probably can align with apollo-android.

Codebear98 · 2019-07-04T11:09:08Z

PR updated following the design of #599 and #602 @designatednerd please take a look, thanks!

designatednerd

Looks like this has a lot of the same stuff in it as #583 - let's get that one dealt with first. I'm going to mark this as changes requested until that gets merged to avoid accidentally merging this one.

Sources/Apollo/GraphQLGETTransformer.swift

Sources/Apollo/HTTPNetworkTransport.swift

Tests/Apollo/Network/HttpNetworkTransportTests.swift

Tests/ApolloCacheDependentTests/StarWarsServerTests.swift

Sources/Apollo/HTTPNetworkTransport.swift

Tests/Apollo/Network/HttpNetworkTransportTests.swift

Sources/Apollo/HTTPNetworkTransport.swift

Tests/ApolloTests/HTTPTransportTests.swift

designatednerd · 2019-07-11T16:23:15Z

One thing that @SirensOfTitan pointed out when I closed his PR is that this ties APQ to the HTTPNetworkTransport - it would probably be good to figure out a way to make this also work for the WebSocketTransport as well.

Codebear98 · 2019-07-15T10:09:33Z

ould probably be good to figure out a way to make this also work for the WebSocketTransport as well.

Probably refactor to decouple the request payload part out from NetworkTransport?
I'm not familiar with ApolloWebSocketClient, does it handle the same way?
Can also update the WebSocketTransport to currently progress.

…er generation

2. remove `_` on private variable 3. add guard clause to optional variable in HttpNetworkTransportTests 4. removed redundant parameter in StarWarsServerTests 5. change // Mark to // MARK 6. single line for method declaration 7. removed redundant code in HTTPNetworkTransport 9. update switch style

…former for SDK lower than 11.

Sources/Apollo/GraphQLGETTransformer.swift

Apollo.xcodeproj/xcshareddata/xcschemes/GitHubAPI.xcscheme

Sources/Apollo/GraphQLHTTPResponseError.swift

designatednerd · 2019-07-18T08:42:00Z

Sources/Apollo/HTTPNetworkTransport.swift

  let url: URL
-  let session: URLSession
+  var session: URLSession


why does this now need to be a var?

Oh, It's used to assign MockSession to intercept the Request.

ok - let's go with this for now. I'm planning to get some stuff allowing a URL session to be passed in with an initializer soon.

Sources/Apollo/HTTPNetworkTransport.swift

Tests/Apollo/Network/AutomaticPersistedQueriesTests.swift

Tests/ApolloCacheDependentTests/StarWarsServerTests.swift

Tests/ApolloTests/GETTransformerTests.swift

RolandasRazma · 2019-07-19T08:40:31Z

Sources/Apollo/HTTPNetworkTransport.swift

-        let response = GraphQLResponse(operation: operation, body: body)
-        completionHandler(response, nil)
+        if self.enableAutoPersistedQueries,
+          let error = body["errors"] as? [JSONObject],


it is getting bit messy here... How about creating GraphQLResponse (else path) and adding method to it to get errors. That way you don't need to know here much about JSONObject and GraphQLResponse already knows about it.

let response = GraphQLResponse(operation: operation, body: body) if response.errors.contain(...) { } else { completionHandler(response, nil) }

good idea, will update accordingly!

renamed useHttpGetMethodForPersistedQueries to useGETForPersistedQueryRetry un-shared test projects fixed grammer typo in GraphQLHTTPResponseError updated to use XCTAssertEqual

RolandasRazma · 2019-07-19T08:53:23Z

Sources/Apollo/HTTPNetworkTransport.swift

+
+    var payload: GraphQLMap = [:]
+
+    if autoPersistQueries {
      guard let operationIdentifier = operation.operationIdentifier else {


not sure if this should be preconditionFailure. You might want persist queries for some but not all request. Wouldn't it be better to just do normal query if operation.operationIdentifier is nil

Thanks for reviewing.
But u can't specify which request to use persistQueries but not all.
And all or none operationIdentifier will be generated according to Apollo run-script.
if operationIdentifier missing and you wanna enable APQs, definitely indicate there is an issue in Apollo Codegen script.

Did I miss something?

--operationIdsPath is path to JSON file that is tooling dependant and/or can be modified as you see fit

The question here is Do we need to support autoPersistQueries just for some requests but not for all?
As the autoPersistQueries seems network traffic optimisation like CDN caching which should be applying to all or none because it depends server side ability to support or not.

I just worry if we don't preconditionFailure for the pre-gen code that's not fulfilling the APQs requirement (but dev opt to turn on APQs). Developers will not be able to get alert about the run script missing operationIdsPath until they look into the traffic logging and find out APQs isn't in place.

In addition, I think the JSON file will not affect the code gen? I mean even you modify the JSON file, it won't affect operation.operationIdentifier in the generated Code?

IIRC the operation identifier is based on the hash of the body, so it shouldn't change based on the location of the codegen.

Personally I'm good with leaving this as a precondition failure - @Codebear98 is correct, if someone wants APQ's and hasn't set up generating operation IDs, we want this to fail loud, hard, and early.

fair enough

designatednerd · 2019-07-29T13:53:37Z

Sources/Apollo/HTTPNetworkTransport.swift

-    guard
-      let delegate = self.delegate,
-      let retrier = delegate as? HTTPNetworkTransportRetryDelegate else {
+  // retryByDefault: still retry if delegate is absent, otherwise repect the value from delegate


this comment should now be removed

designatednerd · 2019-07-29T13:57:46Z

Sources/Apollo/HTTPNetworkTransport.swift

+          _ = self.send(operation: operation, retryFor: error, completionHandler: completionHandler)
+      })
+    } else if retryByDefault {
+      _ = self.send(operation: operation, retryFor: error, completionHandler: completionHandler)


Still have this concern

designatednerd · 2019-07-29T13:58:35Z

Sources/Apollo/HTTPNetworkTransport.swift

    request.setValue("application/json", forHTTPHeaderField: "Content-Type")
+    request.setValue(operation.operationIdentifier, forHTTPHeaderField: "X-APOLLO-OPERATION-ID")


I think you may need to either rebase or merge in from master, there's some stuff there around this that's checking whether there's an operation ID before sending this

yes, and I would also add operationName in APQs request body, as it's available now thanks for 2.16 tooling upgrade!

designatednerd · 2019-07-29T14:02:18Z

Tests/ApolloCacheDependentTests/StarWarsServerTests.swift

+
+class APQsWithGetMethodConfig: TestConfig, HTTPNetworkTransportRetryDelegate{
+  func networkTransport(_ networkTransport: HTTPNetworkTransport, receivedError error: Error, for request: URLRequest, response: URLResponse?, retryHandler: @escaping (Bool) -> Void) {
+    retryHandler(true)


Do you want to check that the received error is of the correct type? Otherwise this could lead to an infinite loop.

designatednerd · 2019-07-29T14:02:57Z

Tests/ApolloTestSupport/MockURLSession.swift

+
+import Foundation
+
+public final class MockURLSession: URLSession {


Does all this need to be public?

designatednerd · 2019-07-29T14:04:13Z

Tests/ApolloTests/GETTransformerTests.swift

-    XCTAssertEqual(url?.absoluteString, "http://localhost:8080/graphql?query=query%20HeroName($episode:%20Episode)%20%7B%0A%20%20hero(episode:%20$episode)%20%7B%0A%20%20%20%20__typename%0A%20%20%20%20name%0A%20%20%7D%0A%7D&variables=%7B%22episode%22:%22EMPIRE%22%7D")
+    let queryString = url?.absoluteString == "http://localhost:8080/graphql?query=query%20HeroName($episode:%20Episode)%20%7B%0A%20%20hero(episode:%20$episode)%20%7B%0A%20%20%20%20__typename%0A%20%20%20%20name%0A%20%20%7D%0A%7D&variables=%7B%22episode%22:%22EMPIRE%22%7D"
+
+    XCTAssertTrue(queryString)


Why was this changed?

designatednerd · 2019-07-29T14:05:27Z

Tests/ApolloTests/GETTransformerTests.swift

+        XCTFail("variables should not nil")
+        return
+      }
+      XCTAssertEqual(variables, "{\"episode\":null}")


you can also check variables.keys.contains("episode") and then check that the value for that is null rather than checking the raw string.

designatednerd · 2019-07-29T14:05:55Z

Tests/ApolloTests/GETTransformerTests.swift

@@ -109,6 +299,6 @@ class GETTransformerTests: XCTestCase {
    let transformer = GraphQLGETTransformer(body: body, url: self.url)

    let url = transformer.createGetURL()
-    XCTAssertNil(url)
+    XCTAssertEqual(url?.absoluteString, "http://localhost:8080/graphql?variables=%7B%22episode%22:%22EMPIRE%22%7D")


I think I wound up deleting this test in master, you're welcome to as well.

designatednerd · 2019-09-13T18:25:28Z

@Codebear98 Do you have interest in continuing this, or do you mind if I take this PR over?

Codebear98 · 2019-09-14T15:47:27Z

@designatednerd sorry, I don’t wanna delay the progress, please take it over if u would, thanks!

designatednerd · 2019-09-16T22:46:28Z

This feature can now be followed at #767

This was referenced Jul 2, 2019

Feature Request: Support for Automatic persisted queries #582

Closed

Add support for Automatic Persisted Queries #376

Closed

Codebear98 force-pushed the support_get_apqs branch 2 times, most recently from 9748016 to 9e69402 Compare July 4, 2019 11:00

designatednerd suggested changes Jul 10, 2019

View reviewed changes

This was referenced Jul 11, 2019

Provide a pattern for dynamic reconfiguration of network transport / auth headers #37

Closed

Add support for Automated Persisted Queries #601

Closed

designatednerd mentioned this pull request Jul 12, 2019

Fix codegen fail for test project and enabled operationIdentifier generation #583

Closed

Henry Hong added 5 commits July 16, 2019 16:22

Fix codegen fail for StarWarsAPI and GitHubAPI, add operationIdentifi…

2c8da2e

…er generation

Fix codegen fail for StarWarsAPI and GitHubAPI, add operationIdentifi…

9fa6349

…er generation

added apqs with get support

48315cd

refactor for fetchOption

8698e81

throw GraphQLHTTPRequestError for JSON serialization error handling

55bb400

Codebear98 force-pushed the support_get_apqs branch from 9e69402 to 55bb400 Compare July 16, 2019 08:27

Henry Hong added 4 commits July 16, 2019 18:55

rename HttpNetworkTransportTests to AutomaticPersistedQueriesTests

3216c59

removed force unwrap in test cases

2e66ece

use url.queryItems to test urlQuery against 2d GraphQLMap in GETTrans…

6d7af3d

…former for SDK lower than 11.

designatednerd suggested changes Jul 18, 2019

View reviewed changes

RolandasRazma reviewed Jul 19, 2019

View reviewed changes

Henry Hong added 2 commits July 19, 2019 16:43

splited MockURLSession to single file

8c6b303

renamed useHttpGetMethodForPersistedQueries to useGETForPersistedQueryRetry un-shared test projects fixed grammer typo in GraphQLHTTPResponseError updated to use XCTAssertEqual

update to use XCTAssertEqual in GETTransformerTests

d51c67a

RolandasRazma reviewed Jul 19, 2019

View reviewed changes

designatednerd added a commit that referenced this pull request Jul 22, 2019

Grab changes from #608 that map all keys and values in dictionary

6da5a32

designatednerd mentioned this pull request Jul 22, 2019

Send operationName #657

Merged

designatednerd reviewed Jul 29, 2019

View reviewed changes

designatednerd mentioned this pull request Sep 13, 2019

Just updated from Apollo 1.8.3 to 2.16.0, codgen:generate no longer works. apollographql/apollo-tooling#1528

Closed

designatednerd mentioned this pull request Sep 16, 2019

Support Auto-Persisted Queries #767

Merged

designatednerd closed this Sep 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Auto Persisted Queries with CDN Support #608

Feature: Auto Persisted Queries with CDN Support #608

Codebear98 commented Jul 2, 2019 •

edited

Loading

designatednerd commented Jul 2, 2019

Codebear98 commented Jul 3, 2019

Codebear98 commented Jul 4, 2019

designatednerd left a comment

designatednerd commented Jul 11, 2019

Codebear98 commented Jul 15, 2019

designatednerd Jul 18, 2019

Codebear98 Jul 19, 2019

designatednerd Jul 29, 2019

RolandasRazma Jul 19, 2019 •

edited

Loading

Codebear98 Jul 19, 2019

RolandasRazma Jul 19, 2019

Codebear98 Jul 19, 2019 •

edited

Loading

RolandasRazma Jul 19, 2019

Codebear98 Jul 23, 2019 •

edited

Loading

designatednerd Jul 23, 2019

RolandasRazma Jul 23, 2019

designatednerd Jul 29, 2019

designatednerd Jul 29, 2019

designatednerd Jul 29, 2019

Codebear98 Jul 31, 2019

designatednerd Jul 29, 2019

designatednerd Jul 29, 2019

designatednerd Jul 29, 2019

designatednerd Jul 29, 2019

designatednerd Jul 29, 2019

designatednerd commented Sep 13, 2019

Codebear98 commented Sep 14, 2019

designatednerd commented Sep 16, 2019

		request.setValue("application/json", forHTTPHeaderField: "Content-Type")
		request.setValue(operation.operationIdentifier, forHTTPHeaderField: "X-APOLLO-OPERATION-ID")


		import Foundation

		public final class MockURLSession: URLSession {

Feature: Auto Persisted Queries with CDN Support #608

Feature: Auto Persisted Queries with CDN Support #608

Conversation

Codebear98 commented Jul 2, 2019 • edited Loading

Usage

Limitation

designatednerd commented Jul 2, 2019

Codebear98 commented Jul 3, 2019

Codebear98 commented Jul 4, 2019

designatednerd left a comment

Choose a reason for hiding this comment

designatednerd commented Jul 11, 2019

Codebear98 commented Jul 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RolandasRazma Jul 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Codebear98 Jul 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Codebear98 Jul 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

designatednerd commented Sep 13, 2019

Codebear98 commented Sep 14, 2019

designatednerd commented Sep 16, 2019

Codebear98 commented Jul 2, 2019 •

edited

Loading

RolandasRazma Jul 19, 2019 •

edited

Loading

Codebear98 Jul 19, 2019 •

edited

Loading

Codebear98 Jul 23, 2019 •

edited

Loading